Apply TIFF Orientation tag (274) on read#1521
Merged
brendancol merged 2 commits intoxarray-contrib:mainfrom May 8, 2026
Merged
Apply TIFF Orientation tag (274) on read#1521brendancol merged 2 commits intoxarray-contrib:mainfrom
brendancol merged 2 commits intoxarray-contrib:mainfrom
Conversation
The reader was ignoring tag 274 and returning the file's stored pixel order regardless of which corner the data was meant to start from. Files written with orientation 2-8 came back as silently flipped or rotated arrays. Parse the tag in `IFD.orientation` (default 1 = top-left, no transform) and apply the TIFF 6.0 spec table after decode in `_apply_orientation`. Orientations 5-8 swap rows and columns, so the GeoTransform's pixel_width and pixel_height swap too -- coords end up with the right size on each axis after the remap. A windowed read on a non-default orientation has ambiguous semantics (does the window refer to file pixels or display pixels?), so the combination raises ValueError rather than silently picking one. Tests cover all eight orientations against a manually-computed expected output, the dim-swap for 5-8, default-tag behaviour, and the window combination raising. tifffile's `imread` does not itself apply the tag, so the comparison target is computed from the source array; that's the only reliable way to verify the spec without depending on a second oriented decoder.
There was a problem hiding this comment.
Pull request overview
This PR fixes GeoTIFF reads for TIFF Orientation tag (274) so that rasters stored with orientations 2–8 are returned in the correct displayed row/column order rather than silently flipped/rotated, addressing the geometry mismatch reported in #1503.
Changes:
- Parse tag 274 in the TIFF header layer as
IFD.orientation(defaulting to 1). - Apply TIFF 6.0 orientation remapping to decoded arrays in the main NumPy read path, and reject
window=reads for non-default orientations. - Add a new test module covering all eight orientations plus windowed-read behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
xrspatial/geotiff/_header.py |
Adds Orientation tag constant and IFD.orientation property (defaults to 1). |
xrspatial/geotiff/_reader.py |
Implements _apply_orientation, applies it in read_to_array, and swaps transform pixel sizes for orientations 5–8. |
xrspatial/geotiff/tests/test_orientation.py |
Adds parameterized tests for pixel remapping, coord shape consistency, and window= rejection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1223
to
+1230
| if orientation != 1: | ||
| arr = _apply_orientation(arr, orientation) | ||
| # Orientations 5-8 swap rows and columns, so the file's stored | ||
| # pixel_width sits on the y-axis of the displayed array and | ||
| # vice versa. Replace the transform with one whose pixel_width | ||
| # comes from the file's pixel_height (and vice versa) so that | ||
| # the returned coords have the right number of entries on each | ||
| # axis. Orientation 5-8 with rotated geographies is rare in |
Comment on lines
+1230
to
+1240
| # axis. Orientation 5-8 with rotated geographies is rare in | ||
| # practice; the sign convention here keeps y decreasing | ||
| # downward, matching the standard top-left-origin convention. | ||
| if orientation in (5, 6, 7, 8): | ||
| t = geo_info.transform | ||
| from ._geotags import GeoTransform | ||
| geo_info.transform = GeoTransform( | ||
| origin_x=t.origin_x, | ||
| origin_y=t.origin_y, | ||
| pixel_width=abs(t.pixel_height), | ||
| pixel_height=-abs(t.pixel_width), |
| # downward, matching the standard top-left-origin convention. | ||
| if orientation in (5, 6, 7, 8): | ||
| t = geo_info.transform | ||
| from ._geotags import GeoTransform |
Comment on lines
+118
to
+126
| def test_orientation_default_unchanged(tmp_path): | ||
| """A file without an Orientation tag defaults to 1 (no transform).""" | ||
| arr = np.arange(24, dtype=np.uint8).reshape(4, 6) | ||
| path = tmp_path / "no_orient.tif" | ||
| tifffile.imwrite(str(path), arr) | ||
|
|
||
| da = open_geotiff(str(path)) | ||
| np.testing.assert_array_equal(da.values, arr) | ||
|
|
… band order, error msg Six review findings on the orientation fix; all addressed in one commit. - ``TAG_ORIENTATION`` now in ``_MANAGED_TAGS`` so it's not collected into ``extra_tags`` on read. Previously, reading an orientation=4 file then writing it back through ``to_geotiff`` re-emitted the original tag and the next read applied the orientation a second time -- doubly-flipped output. Round-trip now stable. - The transform swap for orientations 5-8 used ``pixel_width=abs(t.pixel_height), pixel_height=-abs(t.pixel_width)`` which silently coerced any input to a north-up convention. South-up or west-up files would flip direction. Now does a plain swap that preserves the original signs. For orientations 6/7/8 on georeferenced files the swap is shape-correct but geometrically approximate -- a warning is now emitted so the user knows to verify the GeoTransform. - Single-band slicing (``band=N``) now happens before ``_apply_orientation`` so reorientation runs on a 2D array rather than a multi-band cube that gets sliced afterwards. - Dropped the redundant local ``from ._geotags import GeoTransform`` inside the read path; the symbol is already imported at module scope. - Improved the ValueError for ``window=`` + non-default orientation: includes the actual orientation value and notes that dask-chunked reads (``chunks=...``) hit the same path. - New tests cover: tag 274 not surviving to extra_tags for any of orientations 2-8, a round-trip read+write+read returning the same array (regression for the doubled-application bug), the warning firing on georeferenced 5-8 files, and band selection + orientation returning the expected 2D array.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1503.
The reader was ignoring tag 274. Files written with orientation 2-8 came back as silently flipped or rotated arrays. Pixel values were correct, geometry was wrong.
What changed
_header.py: parse tag 274 asIFD.orientation(default 1)._reader.py:_apply_orientationremaps the decoded array per the TIFF 6.0 table. For orientations 5-8 the row and column axes swap, so the GeoTransform'spixel_widthandpixel_heightswap too, keeping y/x coord shapes consistent with the displayed array._reader.py: a windowed read with non-default orientation raises ValueError. The semantics of "window in file pixels vs. display pixels" are ambiguous and supporting it is its own design call.Tests
23 new tests in
tests/test_orientation.py:imreaddoes not itself apply the tag, so the comparison target is computed from the source array).window=+ non-default orientation raises ValueError;window=+ orientation 1 still works.Test plan
pytest xrspatial/geotiff/tests/test_orientation.py(23 passed)pytest xrspatial/geotiff/tests/(713 passed, 4 skipped, 3 pre-existing matplotlib palette failures unrelated)